Skip to content

Allow third party Zig modules#500

Open
ivansantiagojr wants to merge 4 commits intospiraldb:developfrom
ivansantiagojr:allow-third-party-modules
Open

Allow third party Zig modules#500
ivansantiagojr wants to merge 4 commits intospiraldb:developfrom
ivansantiagojr:allow-third-party-modules

Conversation

@ivansantiagojr
Copy link
Copy Markdown
Contributor

This PR makes a suggestion of feature to allow Ziggy Pydust users to add third party modules.

To achieve this, I add a list of imports to PythonModuleOptions and then iterated this list calling lib.addImport for each module. The same was made for libtest.

I also added basic docs giving an example of how to use that, so feel free to consult it and analyze if this suggestion is a good way to address the issue ziglang/zig#474 .

This commit adds support to use external Zig libraries in Ziggy Pydust by passing a list a of imports to
PythonModuleOptions and calling addImport for each module in the lib and libtest.
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Jun 28, 2025

CLA assistant check
All committers have signed the CLA.

@ivansantiagojr
Copy link
Copy Markdown
Contributor Author

I open this PR as a draft because I am getting this error when trying to use Ziggy Pydust with these modifications, take a look at the Zig error message:

install
└─ install generated to mdz.abi3.so
   └─ zig build-lib mdz ReleaseSafe native
      └─ translate-c failure
error: error: CacheCheckFailed

error: the following command exited with error code 1:
/tmp/tmpkgmgy72_/.venv/lib/python3.13/site-packages/ziglang/zig translate-c -lc --listen=- -OReleaseSafe -I /usr/include/python3.13 -D Py_LIMITED_API=0x030d05f0 /home/ivan/Documents/personal_git/mdz/pydust/src/ffi.h
Build Summary: 1/5 steps succeeded; 1 failed
install transitive failure
└─ install generated to mdz.abi3.so transitive failure
   └─ zig build-lib mdz ReleaseSafe native transitive failure
      └─ translate-c failure
error: the following build command failed with exit code 1:
.zig-cache/o/1cc39b81d4afaad5bf864b1d780902e9/build /tmp/tmpkgmgy72_/.venv/lib/python3.13/site-packages/ziglang/zig /tmp/tmpkgmgy72_/.venv/lib/python3.13/site-packages/ziglang/lib /home/ivan/Documents/personal_git/mdz .zig-cache /home/ivan/.cache/zig --seed 0x7e74b40b -Z698b40c7d38d9e90 install -Dpython-exe=/tmp/tmpkgmgy72_/.venv/bin/python -Doptimize=ReleaseSafe

And I do not know how to fix that properly, maybe I forgot to add the lib somewhere? Or is it just some Zig detail I let pass (given that I am not very good with Zig)? I would appreciate some help, and would love to help implement this feature.

The project I tried to build and got the error above is this one: https://github.com/ivansantiagojr/mdz/tree/using-fork-ziggy-pydust

@ivansantiagojr ivansantiagojr marked this pull request as ready for review July 29, 2025 13:29
@ivansantiagojr
Copy link
Copy Markdown
Contributor Author

ivansantiagojr commented Aug 13, 2025

To register here (and possibly get help from the community), the compilation error described above only happens in the develop branch. When I checkout the last release's version, it builds and works fine, which means I can use third-party Zig modules with the code additions made in this PR from version 0.25.1.

@ivansantiagojr
Copy link
Copy Markdown
Contributor Author

Workaround:

I noticed that in the generated pydust.build.zig it calls the addTranslateC with the root_source_file = b.path("pydust/src/ffi.h"), the thing is: it tries to look for this file in my project instead of Ziggy Pydust's src, so that's why the code does not compile. I could make it work by creating this path in my project and copying the contents of ffi.h to that place.

How could I make the project get this file from the installed Ziggy Pydust instead of my current project? Any ideas @jburgy @robert3005

@jburgy
Copy link
Copy Markdown
Contributor

jburgy commented Aug 27, 2025

@ivansantiagojr your issue is likely related to this discord thread (see this minimal repro if you can't access the discord thread). Note that I raised ziglang/translate-c#47 and found a work-around. Reading this reply (as well as ziglang/zig#24497), I understand now that zig has two translate-c implementations and 0.14 gives the clang-based one by default. You probably don't want to wait until 0.16 to have the zig-based implementation. That leaves you with 2 alternatives:

  1. revert that portion of my change and rely on @cImport
  2. use the new translate-c via build.zig.zon

Let me know what you prefer and how I can help you with it.

@ivansantiagojr
Copy link
Copy Markdown
Contributor Author

Oh, I didn't know there was a discord server. Thanks for letting me know and for the tips! I will investigate a little more and try those alternatives you listed locally, probably another PR should be open to solve that.

@robert3005
Copy link
Copy Markdown
Member

I think if we have #526 we do not need this?

@robert3005 robert3005 closed this Sep 29, 2025
@ivansantiagojr
Copy link
Copy Markdown
Contributor Author

I think if we have #526 we do not need this?

@robert3005, we'll need both, actually. This PR allows us to use third party modules, and #526 only reverts the strategy to use C modules. In order for the code of this branch to work, we need #526 to be merged first. Sorry for not making it clearer earlier!

@robert3005 robert3005 reopened this Sep 29, 2025
@recipe
Copy link
Copy Markdown

recipe commented Feb 2, 2026

I'm using the following workaround until the permanent fix is merged.

robert3005 pushed a commit that referenced this pull request Feb 17, 2026
# Changes

Use `@cImport` because translate-c does not work with files outside of
the source directory, and `pydust.build.zig` depends on it to work.

The original changes were added in the commit
9fb5721, and that also removed
`usingnamespace`, so I was careful I did not re-add those.

# Notes

Discussions about this happened in the PR #500, where I got stuck with
the usage of translate-c, so @jburgy listed 2 ways to fix that in [this
comment
](#500 (comment)).

I decided to revert the change and use `@cImport` again because
[ziglang/translate-c](https://github.com/ziglang/translate-c) does not
have tagged releases and it targes latest Zig, even when unstable. It is
worth keeping an eye on this library, though, we might want to use that
in the future Zig releases.
@jburgy
Copy link
Copy Markdown
Contributor

jburgy commented Feb 21, 2026

@ivansantiagojr I have been noodling around with converting @cImport to .addTranslateC in a different context and I got something that works. Take a look at jburgy/blog#35. I can adapt it to Ziggy Pydust if you think it might help

@ivansantiagojr
Copy link
Copy Markdown
Contributor Author

@ivansantiagojr I have been noodling around with converting @cImport to .addTranslateC in a different context and I got something that works. Take a look at jburgy/blog#35. I can adapt it to Ziggy Pydust if you think it might help

@jburgy I definitely think that using translate C on build.zig is better. However, this branch is already working. I managed to get a fix when reverting to @cImport in #526. Nonetheless, I believe we should adopt addTranslateC to prepare to the next Zig release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants